Skip to content

[WIP, DNM] Enable exit tests on Android API level 28 and newer. #1381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

[WIP, DNM] Enable exit tests on Android API level 28 and newer. #1381

wants to merge 20 commits into from

Conversation

@grynspan
Copy link
Contributor

@grynspan grynspan commented Oct 27, 2025 •
edited
Loading

This PR enables exit tests on Android.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@grynspan grynspan added this to the Swift 6.x (main) milestone Oct 27, 2025
@grynspan grynspan added enhancement New feature or request exit-tests ☠️ Work related to exit tests android 🤖 Android support labels Oct 27, 2025
jakepetroules
jakepetroules reviewed Oct 27, 2025
View reviewed changes
Sources/Testing/ExitTests/SpawnProcess.swift
environment["SWT_CLOSEFROM"] = String(describing: highestFD + 1)
#elseif os(Android)
// Android does not have posix_spawn_file_actions_addclosefrom_np() nor
// closefrom(2), so we don't attempt this operation there.
Copy link

@jakepetroules jakepetroules Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android has close_range (which closefrom is typically trivially implemented in terms of). See swiftlang/swift-subprocess#172

Copy link
Contributor Author

@grynspan grynspan Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close_range() requires an upper bound, which we don't know. (Also setting CLOSE_RANGE_CLOEXEC is a good way to break code that opened a higher fd and doesn't want FD_CLOEXEC set.)

Copy link

@jakepetroules jakepetroules Nov 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that you pass ~0U (max int) as the upper bound, e.g. when reading SWT_CLOSEFROM, call close_range(x, ~0U) instead of closefrom(x). That is how closefrom is implemented internally on the platforms where it exists anyways -- close_range is the syscall, where the kernel can clamp the upper bound down to the actual highest open fd, while closefrom is only in libc.

@finagolfin
Copy link
Member

finagolfin commented Oct 28, 2025

Thanks, Jonathan, will try it and let you know. 👍

@finagolfin
Copy link
Member

finagolfin commented Oct 30, 2025 •
edited
Loading

Seeing a couple build errors because of @available attributes:

/data/data/com.termux/files/home/swift-testing/Sources/Testing/ExitTests/WaitFor.swift:84:58: �[1;31merror: �[1;39m'ExitStatus' is only available in Android 28 or newer�[0;0m
 �[0;36m81 |�[0;0m #elseif SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android)
 �[0;36m82 |�[0;0m /// A mapping of awaited child PIDs to their corresponding Swift continuations.
 �[0;36m83 |�[0;0m private nonisolated(unsafe) let _childProcessContinuations = {
    �[0;36m|�[0;0m                                 `- �[1;39mnote: �[1;39madd '@available' attribute to enclosing let�[0;0m
 �[0;36m84 |�[0;0m   let result = ManagedBuffer<[pid_t: CheckedContinuation<ExitStatus, any Error>], pthread_mutex_t>.create(
    �[0;36m|�[0;0m                                                          |- �[1;31merror: �[1;39m'ExitStatus' is only available in Android 28 or newer�[0;0m
    �[0;36m|�[0;0m                                                          `- �[1;39mnote: �[1;39madd 'if #available' version check�[0;0m
 �[0;36m85 |�[0;0m     minimumCapacity: 1,
 �[0;36m86 |�[0;0m     makingHeaderWith: { _ in [:] }

/data/data/com.termux/files/home/swift-testing/Sources/Testing/ExitTests/WaitFor.swift:106:68: �[1;31merror: �[1;39m'ExitStatus' is only available in Android 28 or newer�[0;0m
�[0;36m102 |�[0;0m ///
�[0;36m103 |�[0;0m /// - Throws: Whatever is thrown by `body`.
�[0;36m104 |�[0;0m private func _withLockedChildProcessContinuations<R>(
    �[0;36m|�[0;0m              `- �[1;39mnote: �[1;39madd '@available' attribute to enclosing global function�[0;0m
�[0;36m105 |�[0;0m   _ body: (
�[0;36m106 |�[0;0m     _ childProcessContinuations: inout [pid_t: CheckedContinuation<ExitStatus, any Error>],
    �[0;36m|�[0;0m                                                                    `- �[1;31merror: �[1;39m'ExitStatus' is only available in Android 28 or newer�[0;0m
�[0;36m107 |�[0;0m     _ lock: UnsafeMutablePointer<pthread_mutex_t>
�[0;36m108 |�[0;0m   ) throws -> R

Not going to spend time fixing this given the bigger SwiftPM issue, but if you want to keep iterating, happy to build it locally for you.

@grynspan
Copy link
Contributor Author

grynspan commented Oct 30, 2025

Thanks, I'll address those and ping you when it's ready to try again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakepetroules jakepetroules jakepetroules left review comments

@stmontgomery stmontgomery Awaiting requested review from stmontgomery stmontgomery will be requested when the pull request is marked ready for review stmontgomery is a code owner

@briancroom briancroom Awaiting requested review from briancroom briancroom will be requested when the pull request is marked ready for review briancroom is a code owner

@jerryjrchen jerryjrchen Awaiting requested review from jerryjrchen jerryjrchen will be requested when the pull request is marked ready for review jerryjrchen is a code owner

Assignees

@grynspan grynspan

@finagolfin finagolfin

Labels

android 🤖 Android support enhancement New feature or request exit-tests ☠️ Work related to exit tests

Projects

None yet

Milestone

Swift 6.x (main)

Development

Successfully merging this pull request may close these issues.

4 participants

@grynspan @finagolfin @jakepetroules